test(dataset-updater): #204 pin RecordsUpdater decode-order + LLM-tool contract (13 tests) - #574
Merged
Merged
Conversation
…l contract (13 tests) Pin the translation-update mechanism surfaced by extending the #204 coverage sweep into the DatasetUpdater module, which had ZERO dedicated tests (no test folder; no reference to RecordsUpdater/UpdateRecord/DecodeValue anywhere in the test project). RecordsUpdater.UpdateRecord is the LLM-facing function-tool wired in DatasetUpdaterConfig (name "UpdateRecord", methodName "UpdateRecord", DatasetUpdaterConfig.cs:488,544-550) — the mechanism by which every translation update lands. DecodeValue (private static) is a 3-pass decode applied to every value before storage: Regex.Unescape then HtmlDecode then UrlDecode, IN THAT ORDER. Two surfaces pinned (output-neutral, additive — no production code changed; DecodeValue stays private, exercised via the public UpdateRecord entry): (1) DecodeValue order-dependence + each-pass coverage: - Plain value passes through unchanged - HtmlDecode pass (< -> <), UrlDecode pass (%3C -> <), Regex.Unescape pass (literal backslash-n -> newline) - Order-dependence: %26amp%3B -> "&" (current HtmlDecode-then-UrlDecode); reversing the two passes yields "&" instead. The order is load-bearing — pinned so a reorder fails loud (a refactor swapping the decode passes would land a different, wrong translation silently). (2) UpdateRecord observable contract: - Empty-target fill (FilledOverwriteCount unchanged) vs filled-target overwrite (counter incremented) — the FilledOverwriteCount is a corruption-risk safety signal - Record-not-found returns "target record not found for pk=X" and mutates nothing - Field-not-found returns "field 'Y' not found in record Z. Available: <keys>" — LLM-facing feedback letting the model self-correct - Return format "{existing}\n==>\n{newValue}" - CallCount increments per call; OverwriteCount accumulates across overwrites; determinism Pure & deterministic given controlled Records. Had ZERO coverage (confirmed). TokenManager excluded from this PR — it is time/state-dependent via DateTime.UtcNow (CleanupTokens, CurrentMinuteTokenCount, WaitForTokenAvailability) and not pinnable without a clock abstraction. Suite 527 passed / 0 failed / 5 skipped (baseline 514 + 13). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Pin the translation-update mechanism for
RecordsUpdater— the LLM-facing function-tool of the DatasetUpdater translation loop, in a module that had ZERO dedicated tests (no test folder; no reference toRecordsUpdater/UpdateRecord/DecodeValueanywhere in the test project).Extends the #204 coverage sweep out of the saturated core pipeline into the newer DatasetUpdater module (merged April 2026 via #210).
Why
RecordsUpdater.UpdateRecordis wired inDatasetUpdaterConfigas theUpdateRecordtool the model calls for every target field of every record (DatasetUpdaterConfig.cs:488,544-550). It is the mechanism by which every translation update lands. It exposes two fragile surfaces:DecodeValue(private static) — a 3-pass decode applied to every value before storage:Regex.Unescape→HtmlDecode→UrlDecode, in that order. The order is load-bearing:%26amp%3Byields&under the current order but&if the UrlDecode/HtmlDecode passes are swapped. A refactor that reordered the passes would land a different, wrong translation silently.UpdateRecordcontract — the LLM-facing error strings ("target record not found for pk=X","field 'Y' not found in record Z. Available: ..."), theFilledOverwriteCountsafety counter (how many updates overwrote a non-empty cell — a corruption-risk signal), and the"old==>new"confirmation the model reads back. A refactor changing any of these silently breaks the model's feedback loop or the safety metric — no exception.Tests added (13)
All output-neutral, additive (no production code changed;
DecodeValuestays private, exercised via the publicUpdateRecordentry with a controlledRecordslist):DecodeValue (5): plain passthrough · HtmlDecode pass (
<→<) · UrlDecode pass (%3C→<) · Regex.Unescape pass (literal\n→newline) · order-dependence%26amp%3B→&pinned (reversing yields&).UpdateRecord contract (8): empty-target fill (counter unchanged) · filled-target overwrite (counter +1) · record-not-found error + records-unchanged · field-not-found error listing available keys · decode applied to stored value · CallCount per call · OverwriteCount accumulation · determinism.
Verification
dotnet test --filter RecordsUpdaterContractTests→ 13/13 passVerify-before-code notes
CleanupTokens/CurrentMinuteTokenCount/WaitForTokenAvailabilityare time/state-dependent viaDateTime.UtcNow; not pinnable without a clock abstraction. Flagged for a future PR if a clock seam is introduced.Context
Cluster degraded (po-2023 down; ai-01 quiet since #565 merge — awaiting jsboige redirect decision). This PR is worker-appropriate: additive test-only, no visual QA, no po-2023 infra, no gate — same profile as #570-#573. The dead-code
ColorExtensionsremoval flagged last tick is held pending ai-01/jsboige steer (production-code change, not my unilateral call); this PR is independent of that.